feat: KSA support with AKS identity bindings #9687

qweeah · 2025-11-27T04:56:51Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds support for Kubernetes Service Account (KSA) based authentication with identity bindings for the ACR credential provider. This enables secret-less ACR image pull via the Kubernetes API server's identity bindings endpoint for token exchange.

Key Changes:

Configuration Enhancement:
- Implemented comprehensive configuration parsing and validation in new cmd/acr-credential-provider/pkg/config package
- Added CLI flags: --ib-sni-name, --ib-default-client-id, --ib-default-tenant-id, --ib-apiserver-ip
Validation Logic:
- Protocol prefix validation: Rejects https:// or http:// prefixes in SNI name
- IP address format validation using net.ParseIP
- Mutual dependency enforcement between SNI name and API server IP
- Validates client/tenant ID requires SNI name to be set
Identity Bindings Token Credential:
- Implemented custom HTTP transport with SNI-based routing to configurable API server IP
- Implemented OAuth2 client credentials flow with service account token exchange
- Refactored to store token, client ID, and tenant ID directly in credential struct for improved performance
Comprehensive Testing:
- Added unit tests for configuration parsing (cmd/acr-credential-provider/pkg/config/identity_bindings_config_test.go) with 100% coverage
- Added unit tests for identity bindings credentials (pkg/credentialprovider/identity_bindings_credentials_test.go)
Code Organization:
- Refactored configuration parsing from main.go to dedicated config package
- Improved separation of concerns between CLI, configuration, and credential logic
- Early resolution of client and tenant IDs from annotations or defaults during credential creation

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

API Server IP Configuration: The implementation uses a configurable API server IP (instead of hardcoded constant) to allow flexibility for different cluster configurations, though most AKS clusters use 10.0.0.1 as the cluster-internal API server address.
SNI-Based Routing: The custom dialer always resolves the SNI hostname to the configured API server IP, ensuring the TLS connection uses SNI for certificate validation while connecting to the correct cluster endpoint.
CA Certificate Rotation: The transport implementation detects CA certificate changes by comparing file content and rebuilds the transport with updated CA pool when rotation is detected.
Validation Strategy: The configuration validation enforces mutual dependencies (SNI name ↔ API server IP) to prevent misconfiguration. Protocol prefixes are rejected during parsing rather than being stripped.
Test Approach: The test suite uses httptest.NewUnstartedServer with self-signed certificates to test the full mTLS flow including custom dialer, TLS configuration, and token exchange logic.
Tenant ID for Future SDK Compatibility: The --ib-default-tenant-id parameter is included from day one to ensure smooth transition if the ACR credential provider switches to using the official Azure SDK in the future.

Does this PR introduce a user-facing change?

ACR credential provider now supports Kubernetes Service Account (KSA) based authentication with identity bindings. Configure via flags: --ib-sni-name, --ib-apiserver-ip, --ib-default-client-id, and --ib-default-tenant-id.

k8s-ci-robot · 2025-11-27T04:57:00Z

Hi @qweeah. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

cmd/acr-credential-provider/main.go

pkg/credentialprovider/identity_bindings_credentials.go

norshtein

LGTM

mainred

I am not familiar with identity binding, but left some comments and hope they help.,

cmd/acr-credential-provider/main.go

pkg/credentialprovider/identity_bindings_credentials.go

nilo19 · 2025-12-01T05:04:43Z

/ok-to-test

Copilot

Pull request overview

This PR adds support for Kubernetes Service Account (KSA) based authentication with identity bindings for the ACR credential provider, enabling secret-less ACR image pull via the Kubernetes API server's identity bindings endpoint for token exchange.

Key changes include:

New identity bindings configuration parsing and validation with CLI flags (--ib-sni-name, --ib-default-client-id, --ib-default-tenant-id, --ib-apiserver-ip)
Custom HTTP transport with SNI-based routing to configurable API server IP
OAuth2 client credentials flow implementation using service account token exchange
Comprehensive test coverage for configuration parsing and credential operations

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`pkg/credentialprovider/identity_bindings_config.go`	Defines the IdentityBindingsConfig struct for identity bindings configuration
`pkg/credentialprovider/identity_bindings_credentials.go`	Implements identityBindingsTokenCredential with custom transport, SNI routing, and OAuth2 token exchange
`pkg/credentialprovider/identity_bindings_credentials_test.go`	Comprehensive unit tests for identity bindings credential creation and token retrieval
`pkg/credentialprovider/azure_credentials.go`	Updates NewAcrProvider to support identity bindings as the highest priority credential type
`pkg/credentialprovider/azure_credentials_test.go`	Updates existing tests to pass empty IdentityBindingsConfig parameter
`cmd/acr-credential-provider/pkg/config/identity_bindings_config.go`	Configuration parsing and validation logic with protocol prefix checks and mutual dependency enforcement
`cmd/acr-credential-provider/pkg/config/identity_bindings_config_test.go`	Unit tests for configuration parsing with 100% coverage of validation scenarios
`cmd/acr-credential-provider/main.go`	Adds CLI flags for identity bindings configuration
`cmd/acr-credential-provider/plugin.go`	Updates ExecPlugin to pass identity bindings config to credential provider
`cmd/acr-credential-provider/plugin_test.go`	Updates test to pass empty IdentityBindingsConfig

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

pkg/credentialprovider/azure_credentials_test.go

pkg/credentialprovider/identity_bindings_credentials.go

pkg/credentialprovider/identity_bindings_credentials_test.go

pkg/credentialprovider/identity_bindings_credentials.go

cmd/acr-credential-provider/main.go

nilo19 · 2025-12-01T05:23:49Z

@qweeah please check UT, linting, etc locally.

pkg/credentialprovider/identity_bindings_credentials.go

nilo19 · 2025-12-03T01:18:22Z

/retest

nilo19 · 2025-12-03T01:20:53Z

Do we have unit tests to cover http proxy case?

qweeah · 2025-12-03T01:46:41Z

Do we have unit tests to cover http proxy case?

Added nil proxy check in the unit test @nilo19

Signed-off-by: Billy Zha <[email protected]>

nilo19 · 2025-12-03T02:21:14Z

/test pull-cloud-provider-azure-e2e-capz

nilo19 · 2025-12-03T11:23:47Z

/lgtm
/approve

k8s-ci-robot · 2025-12-03T11:23:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clnv, mainred, nilo19, norshtein, qweeah

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [nilo19]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 27, 2025

k8s-ci-robot requested review from cheftako and mainred November 27, 2025 04:56

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 27, 2025

github-actions bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 27, 2025

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 27, 2025

norshtein reviewed Nov 27, 2025

View reviewed changes

cmd/acr-credential-provider/main.go Outdated Show resolved Hide resolved

clnv reviewed Nov 27, 2025

View reviewed changes

pkg/credentialprovider/identity_bindings_credentials.go Outdated Show resolved Hide resolved

qweeah force-pushed the qweeah/ksa-ib branch 4 times, most recently from a9fd321 to cfdc927 Compare November 28, 2025 06:12

norshtein approved these changes Nov 28, 2025

View reviewed changes

clnv approved these changes Nov 30, 2025

View reviewed changes

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 1, 2025

mainred reviewed Dec 1, 2025

View reviewed changes

cmd/acr-credential-provider/main.go Show resolved Hide resolved

pkg/credentialprovider/identity_bindings_credentials.go Show resolved Hide resolved

pkg/credentialprovider/identity_bindings_credentials.go Show resolved Hide resolved

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 1, 2025

nilo19 requested a review from Copilot December 1, 2025 05:10

Copilot started reviewing on behalf of nilo19 December 1, 2025 05:10 View session

Copilot finished reviewing on behalf of nilo19 December 1, 2025 05:14

Copilot AI reviewed Dec 1, 2025

View reviewed changes

nilo19 reviewed Dec 1, 2025

View reviewed changes

cmd/acr-credential-provider/main.go Outdated Show resolved Hide resolved

mainred approved these changes Dec 2, 2025

View reviewed changes

nilo19 reviewed Dec 2, 2025

View reviewed changes

pkg/credentialprovider/identity_bindings_credentials.go Outdated Show resolved Hide resolved

nilo19 reviewed Dec 2, 2025

View reviewed changes

pkg/credentialprovider/identity_bindings_credentials.go Show resolved Hide resolved

qweeah force-pushed the qweeah/ksa-ib branch from 99aa6a2 to 9573fba Compare December 3, 2025 00:02

qweeah force-pushed the qweeah/ksa-ib branch from 9573fba to 5818f9e Compare December 3, 2025 01:45

feat: KSA support with AKS identity bindings

b840380

Signed-off-by: Billy Zha <[email protected]>

qweeah force-pushed the qweeah/ksa-ib branch from 5818f9e to b840380 Compare December 3, 2025 01:54

k8s-ci-robot assigned nilo19 Dec 3, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 3, 2025

k8s-ci-robot merged commit b452751 into kubernetes-sigs:master Dec 3, 2025
23 checks passed

feat: KSA support with AKS identity bindings #9687

feat: KSA support with AKS identity bindings #9687

Conversation

qweeah commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

k8s-ci-robot commented Nov 27, 2025

Uh oh!

Uh oh!

Uh oh!

norshtein left a comment

Choose a reason for hiding this comment

Uh oh!

mainred left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nilo19 commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nilo19 commented Dec 1, 2025

Uh oh!

Uh oh!

Uh oh!

nilo19 commented Dec 3, 2025

Uh oh!

nilo19 commented Dec 3, 2025

Uh oh!

qweeah commented Dec 3, 2025

Uh oh!

nilo19 commented Dec 3, 2025

Uh oh!

nilo19 commented Dec 3, 2025

Uh oh!

k8s-ci-robot commented Dec 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

qweeah commented Nov 27, 2025 •

edited

Loading